-
-
Notifications
You must be signed in to change notification settings - Fork 142
GH1173 Experiment with fuller typing #1193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
You'd still need to have the |
I was able to make some progress here, let me know if this is what you envisioned and I will clean it up. |
Hey - yeah, this is what I was thinking - I don't really understand why
it looks like Irv disagrees and that what I've suggested may be against the pandas-stubs philosophy - which is fair enough |
So I looked more carefully at the pandas docs, which do say that the accepted So I'll look more carefully at this PR now with that in mind. |
tests/test_frame.py
Outdated
check( | ||
assert_type( | ||
df.query("col1 > col2", parser="pandas", engine="numexpr"), pd.DataFrame | ||
), | ||
pd.DataFrame, | ||
) | ||
check( | ||
assert_type( | ||
df.query("col1 > col2", parser="pandas", engine="numexpr"), pd.DataFrame | ||
), | ||
pd.DataFrame, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicate tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
I will raise an issue on the pandas side if it is not simpler to have all the arguments directly in the function. It is a good point since it should not be too hard to maintain. |
It's a docs issue, so raise an issue there that it would be more useful to list the arguments under |
Opened the issue on pandas repo, will mark this PR as blocked for now until the pandas team can clarify orientation on this. |
I think we should remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @loicdiridollou
kwargs
inDataFrame.query
according toDataFrame.eval
#1173assert_type()
to assert the type of any return value@MarcoGorelli I took a try at your idea of typehinting the whole set of arguments in
pd.DataFrame.query
but stumbling upon the issue when the user passes a dictionary (which is still an allowed behavior).Wondering if there is something I am missing here, please let me know!